feat(security): add SHA-256 hook integrity verification by playb0t · Pull Request #119 · rtk-ai/rtk

playb0t · 2026-02-14T11:43:27Z

PR: Hook integrity verification via SHA-256

Summary

Adds SHA-256 integrity verification for rtk-rewrite.sh to detect unauthorized modifications. The hook auto-approves all rewritten commands (permissionDecision: "allow"), making it a high-value target for command injection via local file modification.

Motivation

The PreToolUse hook at ~/.claude/hooks/rtk-rewrite.sh bypasses Claude Code's permission prompts for every rewritten command. Any process running as the current user (malicious npm postinstall, compromised dependency, local malware) can overwrite this file and inject arbitrary commands that execute without user awareness.

This is a known attack class - PromptArmor demonstrated identical hook hijacking via marketplace plugins (October 2025), and Cymulate reported CVE-2025-54794/CVE-2025-54795 for related Claude Code command injection vectors (August 2025).

What changed

New: `src/integrity.rs`

compute_hash(path) - SHA-256 of any file, returned as lowercase hex
store_hash(hook_path) - writes .rtk-hook.sha256 alongside hook (sha256sum-compatible format, set to 0o444)
verify_hook() / verify_hook_at(path) - compares stored vs actual hash
remove_hash(hook_path) - cleanup for uninstall
runtime_check() - startup gate for operational commands
run_verify() - handler for rtk verify subcommand
IntegrityStatus enum: Verified, Tampered, NoBaseline, NotInstalled, OrphanedHash
14 unit tests covering all states and edge cases

Modified: `src/init.rs`

ensure_hook_installed() now calls integrity::store_hash() after writing hook + setting permissions
Hash is stored on every init (idempotent), ensuring baseline exists even for pre-existing installs
uninstall() removes hash file alongside hook
show_config() displays integrity status

Modified: `src/main.rs`

Added mod integrity
Added Commands::Verify subcommand
Added is_operational_command() - distinguishes hook-pipeline commands (git, cargo, ls...) from meta commands (init, gain, verify...)
Operational commands run integrity::runtime_check() at startup

Modified: `Cargo.toml`

Added sha2 = "0.10" (RustCrypto, pure Rust, no C dependencies)

Behavior

On `rtk init -g`

RTK hook installed (global).
  Hook:      ~/.claude/hooks/rtk-rewrite.sh
  Integrity: sha256 stored
  ...

On every operational command (e.g. `rtk git status`)

Hook verified: silent, no overhead visible to user

Hook tampered: stderr warning + exit 1

rtk: hook integrity check FAILED
  Expected hash: a1b2c3d4e5f6...
  Actual hash:   f6e5d4c3b2a1...

  The hook at ~/.claude/hooks/rtk-rewrite.sh has been modified.
  This may indicate tampering. RTK will not execute.

  To restore:  rtk init -g --auto-patch
  To inspect:  rtk verify
  To override: RTK_SKIP_INTEGRITY=1 rtk <command>

No baseline (pre-existing install): silent, continue. User upgrades naturally via next rtk init -g
Not installed: silent, continue

`rtk verify` (manual check)

PASS  hook integrity verified
      sha256:a1b2c3d4...
      /home/user/.claude/hooks/rtk-rewrite.sh

Emergency bypass

RTK_SKIP_INTEGRITY=1 rtk git status

Design decisions

Hash stored alongside hook (.rtk-hook.sha256 in same directory) - simple, discoverable, sha256sum-compatible format for manual verification
Read-only permissions (0o444) on hash file - speed bump, not security boundary. Forces attacker to chmod before modification
Fail closed on tampering - operational commands refuse to execute. Correct default for a security control on an auto-approve hook
Silent on NoBaseline - users who installed before this PR aren't blocked. Baseline established on next rtk init -g
Meta commands skip check - init, gain, verify, config don't go through the hook pipeline, no reason to gate them
sha2 crate (RustCrypto) - pure Rust, no C dependencies, well-audited, minimal binary size impact

What this does NOT do

Does not protect against an attacker who modifies both the hook AND the hash file (defense in depth, not absolute prevention)
Does not add cryptographic signing (would require key management - future consideration)
Does not modify the hook script itself (self-verification in bash is bypassable)
Does not add inotifywait/fswatch monitoring (mentioned in advisory as secondary recommendation)

Testing

cargo test integrity       # 14 unit tests
rtk init -g --auto-patch   # Verify hash is created
rtk verify                 # Should show PASS
rtk init --show            # Should show integrity status

# Tamper test:
echo "# tampered" >> ~/.claude/hooks/rtk-rewrite.sh
rtk git status             # Should fail with integrity warning
rtk verify                 # Should show FAIL with hashes
rtk init -g --auto-patch   # Should restore + re-hash
rtk verify                 # Should show PASS again

References

SA-2025-RTK-001 Finding F-01
PromptArmor marketplace plugin hijacking (October 2025)
CVE-2025-54794, CVE-2025-54795 (Cymulate, August 2025)

Add runtime integrity verification for rtk-rewrite.sh to detect unauthorized modifications to the PreToolUse hook. The hook auto-approves all rewritten commands via permissionDecision: "allow", bypassing Claude Code's permission prompts. Any modification by a local process (malicious dependency, compromised npm postinstall, etc.) becomes a command injection vector. Changes: - New module: src/integrity.rs (SHA-256 hash, verify, runtime gate) - rtk init: stores hash after hook installation (idempotent) - rtk verify: new subcommand for manual integrity check - rtk init --show: displays integrity status - rtk uninstall: cleans up hash file - Operational commands check integrity at startup (fail closed) - Emergency bypass: RTK_SKIP_INTEGRITY=1 Ref: SA-2025-RTK-001 (Finding F-01) Ref: PromptArmor hook hijacking (Oct 2025) Ref: CVE-2025-54794, CVE-2025-54795

FlorianBruniaux

Security Review — PR #119: Hook Integrity Verification

Overall: Solid PR that takes RTK from 0 protection to SHA-256 verification. Good lib choice (sha2/RustCrypto), clean enum design, good test coverage (14 tests). Merge-worthy with fixes below.

Summary of findings

#	Severity	Finding
C-02	🔴 CRITICAL	`RTK_SKIP_INTEGRITY=1` bypass — any process can disable verification via env var
I-02	🟡 IMPORTANT	`is_operational_command()` blacklist — fragile for a security boundary
I-03	🟡 IMPORTANT	Hash file parsing too permissive — accepts malformed formats silently
N-01	🟢 NIT	Truncated hash display could panic on malformed input
N-02	🟢 NIT	Error message advertises bypass mechanism to attackers

What's well done

IntegrityStatus enum covers all states explicitly
Fail-closed on Tampered — correct default
Silent on NoBaseline — no breaking change for existing installs
sha256sum -c compatible format — manually verifiable
Integration in init/uninstall/show_config is complete and idempotent

Known limitation (not blocking)

TOCTOU race between RTK verification and Claude Code hook execution is inherent to the architecture. This PR still catches 90%+ of realistic attacks (persistent hook modification). Documented for follow-up.

Recommended action

Fix C-02 (remove or gate the env var bypass) and I-02 (whitelist vs blacklist), then this is good to merge. I-03 and nits can be addressed in this PR or follow-up.

FlorianBruniaux · 2026-02-14T15:42:47Z

src/integrity.rs

+    // Allow emergency bypass
+    if std::env::var("RTK_SKIP_INTEGRITY").unwrap_or_default() == "1" {
+        return Ok(());
+    }


🔴 C-02: RTK_SKIP_INTEGRITY bypass is too permissive

Any process in the environment can disable integrity verification by setting RTK_SKIP_INTEGRITY=1. This includes:

A malicious npm postinstall script

A compromised .envrc or .env file

Any parent process that spawns rtk

Suggested fix — either:

Remove the env var bypass entirely (preferred). If someone needs to override, they should rtk init -g --auto-patch to re-establish the baseline.

If you keep it, require interactive confirmation on a TTY:

if std::env::var("RTK_SKIP_INTEGRITY").unwrap_or_default() == "1" { if atty::is(atty::Stream::Stderr) { eprintln!("rtk: RTK_SKIP_INTEGRITY is set. Skip integrity check? [y/N]"); // read confirmation... } else { // Non-interactive: refuse to skip eprintln!("rtk: RTK_SKIP_INTEGRITY requires interactive confirmation"); std::process::exit(1); } }

Option 1 is simpler and more secure. The bypass creates a false sense of security — if an attacker can modify the hook, they can also set the env var.

FlorianBruniaux · 2026-02-14T15:42:47Z

src/main.rs

+            | Commands::Verify
+            | Commands::Proxy { .. }
+    )
+}


🟡 I-02: is_operational_command() uses a blacklist — should be a whitelist

Current logic: "these meta commands are excluded, everything else gets checked."

Problem: every new command added to RTK is automatically considered "operational" and gets integrity-checked. That's actually the right default today — but the real risk is the inverse: if someone adds a new meta command and forgets to exclude it, it silently gets integrity-checked (not dangerous). However, the blacklist pattern is fragile for a security boundary.

Suggested fix — invert to whitelist:

fn is_operational_command(cmd: &Commands) -> bool { matches!( cmd, Commands::Git { .. } | Commands::Cargo { .. } | Commands::Gh { .. } | Commands::Pnpm { .. } | Commands::Npm { .. } // ... all commands that go through the hook pipeline ) }

This way, new commands are not integrity-checked by default until explicitly added. Safer for evolution: a forgotten command fails open (no check) rather than creating confusion about what's protected.

Alternative (if you prefer keeping blacklist): add a comment explaining the security rationale and a // SECURITY: update this list when adding new meta commands marker.

FlorianBruniaux · 2026-02-14T15:42:47Z

src/integrity.rs

+        .filter(|s| s.len() == 64 && s.chars().all(|c| c.is_ascii_hexdigit()))
+        .map(|s| s.to_string())
+        .with_context(|| format!("Invalid hash format in {}", path.display()))
+}


🟡 I-03: Hash file parsing is too permissive

split_whitespace().next() accepts any format where the first whitespace-separated token is a 64-char hex string. This means these all parse successfully:

abc123... rtk-rewrite.sh # correct sha256sum format abc123... ANYTHING_HERE # wrong filename, still accepted abc123... # no filename at all abc123...\t\t\tgarbage # tabs + garbage

Suggested fix — validate the full sha256sum -c format:

fn read_stored_hash(path: &Path) -> Result<String> { let content = fs::read_to_string(path) .with_context(|| format!("Failed to read hash file: {}", path.display()))?; let line = content.lines().next() .with_context(|| format!("Empty hash file: {}", path.display()))?; // Expect exactly: "<64 hex> <filename>\n" let parts: Vec<&str> = line.splitn(2, " ").collect(); // two-space separator if parts.len() != 2 { anyhow::bail!("Invalid hash format in {} (expected 'hash filename')", path.display()); } let hash = parts[0]; if hash.len() != 64 || !hash.chars().all(|c| c.is_ascii_hexdigit()) { anyhow::bail!("Invalid SHA-256 hash in {}", path.display()); } Ok(hash.to_string()) }

This is defense-in-depth: if an attacker can write the hash file they've already won, but strict parsing prevents accidental corruption from being silently accepted.

FlorianBruniaux · 2026-02-14T15:42:47Z

src/integrity.rs

+        }
+        IntegrityStatus::Tampered { expected, actual } => {
+            eprintln!("rtk: hook integrity check FAILED");
+            eprintln!("  Expected hash: {}...", &expected[..16]);


🟢 Nit: truncated hash display uses hardcoded slice

&expected[..16] will panic if the hash is somehow shorter than 16 chars (shouldn't happen with SHA-256, but defense-in-depth).

eprintln!(" Expected hash: {}...", &expected.get(..16).unwrap_or(&expected)); eprintln!(" Actual hash: {}...", &actual.get(..16).unwrap_or(&actual));

Minor, but avoids a potential panic if read_stored_hash ever returns a malformed value.

FlorianBruniaux · 2026-02-14T15:42:47Z

src/integrity.rs

+            eprintln!();
+            eprintln!("  To restore:  rtk init -g --auto-patch");
+            eprintln!("  To inspect:  rtk verify");
+            eprintln!("  To override: RTK_SKIP_INTEGRITY=1 rtk <command>");


🟢 UX: error message leaks the bypass mechanism

To override: RTK_SKIP_INTEGRITY=1 rtk <command>

This tells an attacker exactly how to bypass the check. If you keep the env var (see C-02 comment), at minimum don't advertise it in the error output. Move it to rtk verify --help or documentation only.

FlorianBruniaux · 2026-02-14T15:42:47Z

src/init.rs

+    // Store SHA-256 hash for runtime integrity verification.
+    // Always store (idempotent) to ensure baseline exists even for
+    // hooks installed before integrity checks were added.
+    integrity::store_hash(hook_path).with_context(|| {


🟢 Good: hash stored on every init, not just on creation

This is the right call — it establishes a baseline even for hooks installed before integrity checks existed. Idempotent and backwards-compatible. 👍

Addresses all findings from FlorianBruniaux's review of PR rtk-ai#119: C-02 (CRITICAL): Remove RTK_SKIP_INTEGRITY env var bypass - Any process in the environment could disable verification - If attacker can modify hook, they can also set env var - Recovery path: `rtk init -g --auto-patch` to re-baseline I-02 (IMPORTANT): Invert is_operational_command to whitelist - Explicitly enumerate commands requiring integrity check - New commands fail open (no check) rather than creating false confidence about protection coverage I-03 (IMPORTANT): Strict hash file parsing - Validate exact sha256sum format: "<64 hex> <filename>" - Reject single-space separator, missing filename, garbage - Prevents silent acceptance of corrupted hash files N-01 (NIT): Safe hash truncation in error output - Use .get(..16).unwrap_or() instead of &[..16] slice - Prevents potential panic on malformed stored hash N-02 (NIT): Remove bypass hint from error message - "To override: RTK_SKIP_INTEGRITY=1" leaked mechanism - Resolved automatically by C-02 (env var removed entirely) Tests: 14 → 16 (+hash_only_no_filename, +wrong_separator)

playb0t · 2026-02-15T13:43:59Z

All findings addressed in the latest commit:

C-02: Removed RTK_SKIP_INTEGRITY env var bypass entirely. Recovery path is rtk init -g --auto-patch.
I-02: Inverted is_operational_command() to whitelist pattern. New commands fail open (no check) rather than false confidence.
I-03: Strict read_stored_hash() - validates two-space separator, 64 hex chars, filename presence. Two new tests added.
N-01: &expected[..16] → .get(..16).unwrap_or(&expected).
N-02: Resolved via C-02 - bypass hint no longer exists in error output.

Tests: 15 passed (16th is #[cfg(unix)], skipped on Windows build, will pass in CI).

FlorianBruniaux requested changes Feb 14, 2026

View reviewed changes

pszymkowiak added the awaiting-changes label Feb 15, 2026

playb0t requested a review from FlorianBruniaux February 18, 2026 08:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

feat(security): add SHA-256 hook integrity verification#119

feat(security): add SHA-256 hook integrity verification#119
playb0t wants to merge 2 commits intortk-ai:masterfrom
playb0t:feat/hook-integrity

playb0t commented Feb 14, 2026

Uh oh!

FlorianBruniaux left a comment

Uh oh!

FlorianBruniaux Feb 14, 2026

Uh oh!

FlorianBruniaux Feb 14, 2026

Uh oh!

FlorianBruniaux Feb 14, 2026

Uh oh!

FlorianBruniaux Feb 14, 2026

Uh oh!

FlorianBruniaux Feb 14, 2026

Uh oh!

FlorianBruniaux Feb 14, 2026

Uh oh!

playb0t commented Feb 15, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

playb0t commented Feb 14, 2026

PR: Hook integrity verification via SHA-256

Summary

Motivation

What changed

New: src/integrity.rs

Modified: src/init.rs

Modified: src/main.rs

Modified: Cargo.toml

Behavior

On rtk init -g

On every operational command (e.g. rtk git status)

rtk verify (manual check)

Emergency bypass

Design decisions

What this does NOT do

Testing

References

Uh oh!

FlorianBruniaux left a comment

Choose a reason for hiding this comment

Security Review — PR #119: Hook Integrity Verification

Summary of findings

What's well done

Known limitation (not blocking)

Recommended action

Uh oh!

FlorianBruniaux Feb 14, 2026

Choose a reason for hiding this comment

🔴 C-02: RTK_SKIP_INTEGRITY bypass is too permissive

Uh oh!

FlorianBruniaux Feb 14, 2026

Choose a reason for hiding this comment

🟡 I-02: is_operational_command() uses a blacklist — should be a whitelist

Uh oh!

FlorianBruniaux Feb 14, 2026

Choose a reason for hiding this comment

🟡 I-03: Hash file parsing is too permissive

Uh oh!

FlorianBruniaux Feb 14, 2026

Choose a reason for hiding this comment

🟢 Nit: truncated hash display uses hardcoded slice

Uh oh!

FlorianBruniaux Feb 14, 2026

Choose a reason for hiding this comment

🟢 UX: error message leaks the bypass mechanism

Uh oh!

FlorianBruniaux Feb 14, 2026

Choose a reason for hiding this comment

🟢 Good: hash stored on every init, not just on creation

Uh oh!

playb0t commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

New: `src/integrity.rs`

Modified: `src/init.rs`

Modified: `src/main.rs`

Modified: `Cargo.toml`

On `rtk init -g`

On every operational command (e.g. `rtk git status`)

`rtk verify` (manual check)

🔴 C-02: `RTK_SKIP_INTEGRITY` bypass is too permissive

🟡 I-02: `is_operational_command()` uses a blacklist — should be a whitelist

🟢 Good: hash stored on every `init`, not just on creation

playb0t commented Feb 15, 2026 •

edited

Loading